Wrap SIGN() in CAST(... AS int) for SQL Server#38260
Open
roji wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes SQL Server translation of Math.Sign(...)/MathF.Sign(...) to avoid InvalidCastException during materialization by ensuring the SQL SIGN(...) result is explicitly cast to int when the input type would otherwise cause SIGN to return a non-int SQL type.
Changes:
- Update SQL Server math translation to wrap
SIGN(...)in a cast-to-int(except when the argument is alreadyint). - Expand the spec test coverage for
Signto include both predicate and scalar projection cases, and add dedicated tests fordecimalandint. - Update provider-specific translation tests (SQL Server/Cosmos) and mark unsupported cases as translation failures for SQLite.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.FunctionalTests/Query/Translations/MathTranslationsSqlServerTest.cs | Updates expected SQL to include CAST(SIGN(...) AS int) and adds assertions for scalar projections plus new decimal/int cases. |
| test/EFCore.Sqlite.FunctionalTests/Query/Translations/MathTranslationsSqliteTest.cs | Updates expected SQL for scalar projections and marks Sign_decimal/Sign_int as unsupported translations for SQLite. |
| test/EFCore.Specification.Tests/Query/Translations/MathTranslationsTestBase.cs | Expands Sign coverage to include scalar projections and adds new Sign_decimal/Sign_int tests. |
| test/EFCore.Cosmos.FunctionalTests/Query/Translations/MathTranslationsCosmosTest.cs | Adds SQL assertions for scalar projections and new decimal/int cases. |
| src/EFCore.SqlServer/Query/Internal/Translators/SqlServerMathTranslator.cs | Implements the cast-to-int wrapping for SIGN(...) to match Math.Sign’s CLR return type. |
auto-merge was automatically disabled
May 12, 2026 18:18
Pull request was converted to draft
roji
force-pushed
the
main-sxs-20260512-184746-copilot-claude-opus-4-6
branch
2 times, most recently
from
0c628ac to
3544f81
Compare
T-SQL SIGN() returns the same type as its input, but Math.Sign/MathF.Sign always return int. Without an explicit CAST, materialization throws InvalidCastException when the argument is decimal, double, or float. Add a dedicated TranslateSign helper that creates the SIGN function node with the argument's own type mapping (reflecting T-SQL behavior), then wraps it in a CAST to int. This ensures SqlExpressionSimplifyingExpressionVisitor does not strip the CAST as a same-type no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roji
force-pushed
the
main-sxs-20260512-184746-copilot-claude-opus-4-6
branch
from
3544f81 to
dd4599b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
T-SQL SIGN() returns the same data type as its input, but Math.Sign() always returns int. When the input is decimal/double/float/long/short/sbyte, the materializer reads the column with GetInt32() and throws InvalidCastException. Fix by wrapping the SIGN() call in CONVERT(int, ...).
Fixes #38249